-
Notifications
You must be signed in to change notification settings - Fork 348
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Wildcard subdomains v2 - e.g. *.google.com #2352
base: main
Are you sure you want to change the base?
Conversation
@mckenfra I can't thank you a 1000 times for your work!! |
@groovecoder can you please review and accept this PR? It is working great. |
Will this work with multi-level subdomains? I wonder if it will do what I've been searching for. I use these containers for local, staging and live sites. Mainly cause they have an elegant line on the tabs. But I always have to select. "Always open site in..." so it remembers the specific client site. Does the following work: |
No wildcard is needed for the ports no. For me, it's always the same port. This is a God-sent! I happened to come here today to see if the code was something I can edit and modify for my needs. Little did I know there would be a PR today lol Thanks for the hard work. I'll prolly try it today after I get free. |
First of all, @mckenfra thanks a lot for reviving this idea. I'd definitely like to see this feature implemented in MAC. However, I have a few UX concerns over this implementation.
For 1, you could explore a solution that use a tooltip when you hover over the For 2, we could change the |
Totally liking that 2e part, will make a lot more sense! love to see this merged with the changes purposed by @dannycolin |
I support:
But for 1, I see personally no need, because I thing that most people who are aware of privacy do have technical knowledge about domains. |
For Mozilla, privacy is for everyone. It's at the core of Mozilla's manifesto. Empowering users, whatever their level of knowledge, is something we should strive to especially when it's very easy to implement like the proposition I made. It's even make more sense to do that small extra step since it's an addon managed by Mozilla itself. If it was a third-party addon, I'd be less concerned. |
@dannycolin Thanks a lot for your feedback 👍 Regarding your suggestion no. 2, see video below. Is this better? Wildcard2.mov |
I would more do it with a |
@bruvv In the current implementation of this pull request, It will not match Is that the behaviour you would expect? |
@mckenfra to respond on your last question i like hat behavior. |
No |
@bruvv Yes I take your point about adding the leading dot. Yes the behaviour is that |
@bruvv The red colour was used so that it stands out clearly, in case people click the subdomain accidentally... but I will be happy to change this to a different colour if you can suggest one? Maybe blue? Or the same colour as the container icon for that particular container? |
I was talking about the visualization not the functionalist. Functional it is perfect, visually it is just missing the |
See below a screencast showing the following changes:
(I've edited the video to remove the steps where I changed the container icon colour each time) Wildcard3.mov |
@mckenfra awesome! I love it. Now hopefully this can be merged. |
@bruvv Great! If there is general consensus on the above design, I'll update the PR to include these UI changes. |
@mckenfra the last design is even better then your first proposal. I agree with it. |
efc9847
to
83fd73b
Compare
Hopefully @dannycolin agrees too (shameless ping :P ) |
… far fewer extra async storage requests (if any)
@bakulf can you review this? |
Hey folks, Since a lot of you have pinged the developers for a review, I just wanted to give you a quick updates on why it's taking more time then initially planned to get this reviewed and merged. Firstl, we're working on a fast and robust way to detect the domain TLDs based on the publicsuffixlist. This takes more time because:
I won't go more into details but I'm sure you can appreciate that this solutions aren't simple as copy/pasting code in the repo :). Second, we needed to work on the UX of this feature to make sure it's as easy as possible to use and so to limit the chance a user shoot themselve in the foot by setting it wrong because the UI was confusing. I'm sharing below a screenshot of what it could look like. Keep in mind that it may not cover all the use cases. This is done on purpose. We will most likely implement a basic feature that do something like "Enable everything or nothing from domain X". This doesn't mean that more advanced features won't be implemented but simply that we prefer to do it incrementally so it has more chances the maintainers of this addon will accept to merge it. |
@mckenfra Just a quick question. I think it has something to do with the following code in
|
@BPower0036 You should only need to add |
@mckenfra Thanks for that! It works! I expected wrongfully to work the other way around, which is actually not possible. |
Hi, I've tried this version 8.0.7.4-WildcardContainers, but no luck.
If I can somehow do any diagnostics, please guide me what to do. Thanks. Please see the screenshots. |
@Hermholtz I tried to make it clear on the Release page that this Wildcard Subdomains build is only a temporary proof-of-concept build - i.e. for testing purposes only! I also stated on the Release page:
So it is expected that none of your sites are visible in this temporary Wildcard Subdomains build. Perhaps you should just uninstall the Wildcard Subdomains build? There is little point spending time configuring all your sites again in the Wildcard Subdomains build, because when Mozilla (hopefully at some point) releases an official update to include the wildcard subdomains feature, none of those sites from this temporary build will be carried over. However, you must be very careful that you uninstall the correct Addon. If you accidentally uninstall the official Mozilla Multi-Account Containers addon, you will lose all of your sites configuration! So when you are on the Firefox Addons page, make sure you delete the addon called I hope that makes sense! |
OK Thank you for clarification, I really did not read the entire documents, I was so eager to try I immediately jumped to installing it :) Now it makes sense. I've uninstalled your temp build. I hope Mozilla can make it working soon, this entire issue is more or less 5 years old already... |
No problem! And I can see from your screenshot that you must have spent a lot of time adding many many sites to your Google container. So this wildcard subdomains feature is going to be a big help for you as well as others like you have been doing the same thing! |
Bug 1315558 was mentioned as supporting this feature. |
Does this mean that I cannot map |
@celluj34 You cannot map |
Can they? I myself don't have a use case for it but I'm sure someone does. Perhaps that's out of the scope of this immediate feature. |
@celluj34 Yes sorry I meant the way it is currently coded, |
@celluj34 It is quite tricky case for regular users. Maybe there is no need to implement support for it. This case could be handled by using Containerise add-on together with MAC - it provide possibility to use RegExp to handle opening in dedicated containers. |
Since this issue only let you trigger the wildcard by flipping a switch, I'd say it's out-of-scope. However, it'd fit perfectly in the PR to manually add an URL. This PR could add support for regular expressions. |
What's status on this PR? I would love to see this merged / released. |
We decided to implement this feature only if we have access to the PublicSuffixList shipped by the browser For this to happen, we need to implement a new API (bug 1315558). However, both @mckenfra and I are working on this on our free time as volunteers so we can't promise you when this is going to be merged. Sorry for not having better news. |
This comment was marked as off-topic.
This comment was marked as off-topic.
Why can't the addon be released by providing its own |
@celluj34 See w3c/webextensions#231 (comment). I won't go into the technical details because that'd be too off-topic but tl;dr inconsistencies between the list shipped by the browser and the addon could have potential security impacts. |
This comment was marked as off-topic.
This comment was marked as off-topic.
Today I was alarmed to discover that my default container has been logged in to Google for I don't know how long. I tried to add a wildcard + apex "always open this container in" rule and found it's not implemented, and then found this PR.
I agree that shipping a hard-coded suffix list in the extension that doesn't come from somewhere more authoritative is a non-starter, but why wait to merge this PR as-is? It would improve MAC and thus user safety today for everybody who uses e.g. Google. Adding support later for the public suffix list will improve the experience even further, but I don't see why that is blocking, especially since this PR is already in a shippable state. |
See pull request #1500 and issue #473
This is a second attempt at implementing a wildcard subdomains feature. This time round, I have tried to keep the amount of code that is changing to the bare minimum. This should make code review easier and minimise any risk of introducing regressions. We can potentially cancel the previous pull request if this one looks better.
MAC.mov
How the code works
When you click the
www
part ofwww.google.com
in the popup, awildcardHostname
property with valuegoogle.com
is added to the storedsiteSettings
object forwww.google.com
inassignManager.js
See code hereIf you then open a tab and navigate to
mail.google.com
, we need the code to find the matchinggoogle.com
wildcard hostname stored in thewww.google.com
site. This is achieved by storing an additional map ofwildcardHostname
values tositeStoreKeys
. See code hereSo when handling the request for
mail.google.com
, the wildcard hostname map is queried to see if there is a mapping formail.google.com
, thengoogle.com
(and then theoreticallycom
). It finds thatgoogle.com
is mapped towww.google.com
, and so uses thewww.google.com
site settings to handle the request. See code hereSyncing
The sync feature should just work as expected, since the new
wildcardHostname
property ofsiteSettings
will be synced along with all the other properties of thesiteSettings
object.Try it out
I have put up a temporary build here for you to try out the feature without having to build it yourself. Let me know if it looks good or if you find any issues.
I have also included unit tests in this pull request.